Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rollbar #871

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add rollbar #871

wants to merge 1 commit into from

Conversation

nikolai-b
Copy link
Contributor

Adds error logging in Rollbar. After adding this to another project I realised it wasn't much work in R. The other logging I tried to add never seemed to work as the R server logs weren't in quite the correct format. I've added the Renviron and rollbar package to all the servers in preparation.

Adds error logging in Rollbar
@Robinlovelace
Copy link
Member

The idea being that any error messages emanating from R inside the app will get reported, right? I've glanced over the minimal documentation in the rollbar R package. One thing to note: the package was last updated 5 years ago so doesn't seem to be actively maintained. If it works to allow named people to see the logs it's a 👍 from me although I cannot at this stage see the advantage compared with the output of /var/log/shiny-server/*.log files. Good article on debugging shiny apps: https://shiny.rstudio.com/articles/debugging.html

On a different note while searching for that I came across this, may also log errors but not sure: https://github.com/dreamRs/shinylogs

Looks fine but a little more info would be useful. Is it the ease with which we can see logs on the rollbar website, which allows authentication via GitHub? I recall you set up a similar system already but cannot recall - did it have 'dog' in the name?

@nikolai-b
Copy link
Contributor Author

I agree. I tried to get /var/log/shiny-server/*.log files to be pushed somewhere a year or two ago but I failed. Didn't want to put much effort into it. The issue with the logs being on the server is you have to actively log in to see problems. A service like rollbar will send notifications (which can be customised etc.) so proactively alerting us. If we had the full logs sent somewhere we'd still want to set up custom alerts as most log lines shouldn't trigger a notification. I'll look at the other package but it is a full error handeling Saas that I wanted. Thanks for checking the package age btw

@Robinlovelace
Copy link
Member

To clarify, what is the next step on this that you're suggesting @nikolai-b ? Package age alone doesn't need to be a blocker but wanted to explore options before reviewing this.

@nikolai-b
Copy link
Contributor Author

nikolai-b commented Mar 29, 2021 via email

@Robinlovelace
Copy link
Member

I think there is no chance of issues with this and generally think moving things forward is good. Given low risk and minimal change to code base will review with 👍 now.

@Robinlovelace
Copy link
Member

Does rollbar need to be added in the install.packages() bit also?

@nikolai-b
Copy link
Contributor Author

Does rollbar need to be added in the install.packages() bit also?

I did it before as I wanted the installed packages to raise an error that will be caught (assuming rollbar works) but we could probably rejig it if you prefer.

@Robinlovelace
Copy link
Member

I did it before as I wanted the installed packages to raise an error that will be caught (assuming rollbar works) but we could probably rejig it if you prefer.

Was wondering how to ensure an error message is generated. But not sure that

library(rollbar)
#> Error in library(rollbar): there is no package called 'rollbar'

Created on 2021-03-29 by the reprex package (v1.0.0)

Is a particularly useful error message if we're looking to test the package!

@nikolai-b
Copy link
Contributor Author

nikolai-b commented Mar 29, 2021

You'll need to install the package and use the correct Renviron with our ROLLBAR_ACCESS_TOKEN and R_ENV if you want to test the error capturing. I'll add you to the rollbar project. I purposely didn't add the package to either
available_locally_pkgs or must_be_installed_pkgs in case code in there has errors. For error handling it made sense to have that code run first. This is likely to need some tweaking as when I run shiny from Rstudio it is in interactive mode so rollbar doesn't handle errors but we can tweak how rollbar attaches if your happy for it to be included.

@Robinlovelace
Copy link
Member

This is good to go I think @nikolai-b.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants